Optimize XPath step by tompng · Pull Request #315 · ruby/rexml

tompng · 2026-05-21T13:11:49Z

Refactor step so that nodeset materialization is deferred. Instead of building the full nodeset up front and filtering through predicates,
each axis returns a scan descriptor ([generator_name, generator_argument]), and step picks a scan strategy based on the predicates' shape.

Predicates are classified into three groups:

kind	examples	strategy passed to the generator
position-independent	`[@a="1"]`, `[name()="foo"]`, `[@a=@b]`	`:uniq` — emit deduplicated matching nodes
simple positional	`[N]`, `[position()=N]`, `[position()>N]`, `[position()<N]`	`[op, value]` — positional scan with one comparison
complex / position-dependent	`[position()*@a]`, `[last()-1]`, ...	`:nodesets` — fall back to per-anchor nodesets + the previous `evaluate_predicate` pipeline

Mixed predicate lists are split: position-independent predicates before the first positional predicate are folded into the node test;
predicates after it are applied per-node on the result.

Each axis can implement zero, one, or all of the three strategies. If a strategy is not implemented, the generator falls back to producing
:nodesets and the common slow path (non_optimized_raw_nodesets_select) handles dedup / positional filtering on flattened nodesets — i.e. the
same behavior as before this PR.

This pull request adds fast paths for:

descendant / descendant-or-self: :uniq (single DFS with a seen-set; this is what speeds up //a//a//a//a)
ancestor / ancestor-or-self: :uniq (parent-chain walk with a seen-set)
preceding-sibling / following-sibling: :uniq and [op, value] (sibling scan with anchor-index tracking)

Other axes (child, parent, self, attribute) keep the previous behavior via the fallback path; they can be optimized in
follow-ups without changing call sites.

Detail

For //a//a//a style queries, the previous code built nodesets keyed by each anchor, including the same descendant once per anchor. The new
:uniq path scans every node at most once per step.

For [position() > N] style predicates on wide trees (e.g. //a/preceding-sibling::*[position()>2]), we previously built the full
preceding-sibling nodeset for each anchor and then ran evaluate_predicate. The new [op, value] path scans children once per parent and uses
anchor-index bookkeeping to recover per-anchor positions.

Note: general XPath cannot be linear — e.g. *[position() * number(@a) % number(@b) = 1] is genuinely O(n²) — so the goal is only to fast-path
position-independent and simple-positional predicates.

Benchmark

DEPTH = 500
xml   = '<a>' * DEPTH + '</a>' * DEPTH
doc   = REXML::Document.new(xml)
WIDTH = 1000
xml_wide = '<root>' + '<child/>' * WIDTH + '</root>'
doc_wide = REXML::Document.new(xml_wide)

REXML::XPath.match(doc, "//a//a");
# processing time: 30.756939s → 0.126807s

REXML::XPath.match(doc_wide, "//*/preceding-sibling::*[position()=10]");
# processing time: 2.446333s → 0.083954s

Also fixes #25

Copilot

Pull request overview

This PR refactors XPath step evaluation in REXML::XPathParser to defer nodeset materialization and introduce axis “scan strategies” that can fast-path common predicate shapes (position-independent and simple positional predicates), improving performance for deep and wide-tree queries.

Changes:

Reworked step to drive axis scans via [scanner_method, scanner_argument] and choose scanning strategies (:uniq, [op, value], :nodesets) based on predicate classification.
Added optimized scanners for descendant(-or-self), ancestor(-or-self), and sibling axes, plus a shared fallback selection path.
Added/updated XPath predicate and sibling-axis tests, including float literal predicates and positional comparisons.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
lib/rexml/xpath_parser.rb	Implements deferred scanning/strategy selection in `step`, predicate classification, and optimized axis scanners.
test/xpath/test_predicate.rb	Adds coverage for float-literal positional predicates and variable-as-position predicates.
test/xpath/test_base.rb	Adds test for `following-sibling::*` with `position() < N` and adjusts nested-predicate test behavior.
test/xpath/test_axis_preceding_sibling.rb	Adds tests for `preceding-sibling::` with `<` and `<=` position predicates.

Comments suppressed due to low confidence (1)

lib/rexml/xpath_parser.rb:834

descendant axis fast path calls raw_node.children when include_self is false without checking the node type or whether children exists. If the context node is not an element/document (e.g., an attribute node), this will raise NoMethodError; previously it would just yield an empty descendant set. Guard this branch so it only iterates children for element/document nodes (or use the same node_type check as in recursive).

        raw_nodes.each do |raw_node|
          if include_self
            recursive.call(raw_node)
          else
            raw_node.children.each(&recursive)
          end

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    def preceding_following_sibling(raw_nodes, tester, selector, reverse:)
+      raw_nodes = raw_nodes.select(&:parent)
+      case selector
+      when :uniq
+        raw_nodes.group_by(&:parent).flat_map do |parent, sibling_nodes|
+          sets = {}.compare_by_identity


Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

+            result = evaluate_predicate(predicate_expr.dclone, [result]).first || []
+          end


+    end
+
+    def preceding_following_sibling(raw_nodes, tester, selector, reverse:)
+      raw_nodes = raw_nodes.select(&:parent)


Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

+          # For xpath like `(/path[predicate1][predicate2])[predicate3][predicate4]`,
+          # add a separator mark to distinguish predicates of the inner parentheses and the outer parentheses.
+          parsed.push(:self, :node) if n[0] == :document || n[0] == :child


+    # If there are no position-based predicates,
+    # return [position_independent_predicates, nil, [], nil]
+    # If there are only one simple position-based predicate,
+    # return [position_independent_predicates, position_operator, position_independent_predicates, nil]


If a predicate of xpath is position-independent, we don't need to create nodesets that has many duplicated nodes with different positions.

Copilot AI review requested due to automatic review settings May 21, 2026 13:11

Copilot started reviewing on behalf of tompng May 21, 2026 13:12 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

tompng force-pushed the xpath_step_optimize branch from 6f94f2e to c4a1440 Compare May 21, 2026 15:57

Copilot AI review requested due to automatic review settings May 21, 2026 16:00

tompng force-pushed the xpath_step_optimize branch from c4a1440 to 67e3270 Compare May 21, 2026 16:00

Copilot started reviewing on behalf of tompng May 21, 2026 16:01 View session

tompng force-pushed the xpath_step_optimize branch from 67e3270 to 593620b Compare May 21, 2026 16:06

Copilot AI reviewed May 21, 2026

View reviewed changes

tompng force-pushed the xpath_step_optimize branch from 593620b to 65c5103 Compare May 21, 2026 16:31

Copilot AI review requested due to automatic review settings May 21, 2026 17:11

tompng force-pushed the xpath_step_optimize branch from 65c5103 to 15a8f52 Compare May 21, 2026 17:11

Copilot started reviewing on behalf of tompng May 21, 2026 17:11 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

tompng marked this pull request as ready for review May 21, 2026 17:26

Optimize XPath step

020085a

If a predicate of xpath is position-independent, we don't need to create nodesets that has many duplicated nodes with different positions.

tompng force-pushed the xpath_step_optimize branch from 15a8f52 to 020085a Compare May 21, 2026 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize XPath step#315

Optimize XPath step#315
tompng wants to merge 1 commit into
ruby:masterfrom
tompng:xpath_step_optimize

tompng commented May 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		result = evaluate_predicate(predicate_expr.dclone, [result]).first \|\| []
		end

Conversation

tompng commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Detail

Benchmark

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tompng commented May 21, 2026 •

edited

Loading